revert(bricks): remove SF colour border/box-shadow fixes by jackgranatowski · Pull Request #212 · codeslash-dev/SLASHED

jackgranatowski · 2026-06-03T18:44:55Z

Summary

Reverts the three SF colour border/box-shadow fix commits (93da924, cafa170, 18303c0) that were introduced to handle event-ordering issues with Bricks' click-outside handler
The mousedown-based approach stopped working after a Bricks Builder version update
Removes all related changes: data-cp-pick-var attribute on swatch buttons, direct addEventListener in onMount, suppress flag in pick(), and SF button mousedown handler in color-swatches.js
Rebuilds editor-app/assets/app.js to match the reverted source

Why

The fix relied on firing our mousedown handler at element level (before Bricks' document-level click-outside handler), but a Bricks update changed the event handling in a way that broke this approach. The border/box-shadow SF colour selection will be handled manually.

Test plan

SF colour panel opens normally for text and background controls
Selecting a colour from the SF panel applies it correctly for text/background
Border and box-shadow controls: SF colour panel opens; note that apply will show "input not found" (expected — this is the known limitation being reverted to)

Generated by Claude Code

Summary by CodeRabbit

Refactor
- Streamlined color picker event handling and interaction flow for improved efficiency.
- Optimized swatch button behavior implementation.
- Simplified color input integration logic.

coderabbitai · 2026-06-03T18:45:02Z

📝 Walkthrough

Walkthrough

ColorPanel picker interaction is refactored from per-button dataset suppression flags to root-level mousedown propagation stopping. Component-owned event binding and pick() suppression logic are removed. All swatch button data-cp-pick-var attributes are removed across panel sections and the ColorSwatch component. Integration button mousedown listener is removed.

Changes

Color picker event handling simplification

Layer / File(s)	Summary
ColorPanel core event handler refactor `plugins/SLASHED-for-WP/integrations/bricks/editor-app/src/components/ColorPanel.svelte`	Remove panelEl reference binding and onMount mousedown handler that supplied onPickValue calls with dataset suppression logic. Simplify pick() by removing the conditional branch that detected and cleared button suppression flags. Replace panel root onclick propagation stop with onmousedown propagation stop.
Remove data-cp-pick-var from swatch buttons `plugins/SLASHED-for-WP/integrations/bricks/editor-app/src/components/ColorPanel.svelte`, `plugins/SLASHED-for-WP/integrations/bricks/editor-app/src/components/ColorSwatch.svelte`	Remove data-cp-pick-var attribute from all swatch buttons in ColorPanel (Quick Use, Brand/Status base/scale/alpha sections) and ColorSwatch component while preserving onclick handlers that invoke pick().
Integration button mousedown removal `plugins/SLASHED-for-WP/integrations/bricks/editor-app/src/lib/color-swatches.js`	Remove mousedown event listener that stopped propagation and prevented default behavior from the injected SF button. Click and keydown handlers remain for panel interaction.

Possibly related PRs

codeslash-dev/SLASHED#201: Both PRs modify ColorPanel.svelte event handling to use root-level stopPropagation to prevent outside-close behavior.
codeslash-dev/SLASHED#208: Both PRs modify ColorPanel.svelte and ColorSwatch.svelte picker wiring by removing data-cp-pick-var and changing mousedown handling.
codeslash-dev/SLASHED#206: Both PRs modify ColorPanel.svelte and color-swatches.js mousedown propagation/preventDefault logic to manage Bricks popover focus.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title 'revert(bricks): remove SF colour border/box-shadow fixes' accurately describes the main change: reverting commits that implemented SF colour selection fixes and removing related code artifacts.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch claude/bricks-border-sf-color-bug-f33Qj

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Reverts commits 93da924, cafa170, and 18303c0. The mousedown-based approach to beat Bricks' click-outside handlers did not work reliably after a Bricks update. Removing all three fix commits so the border/box-shadow SF colour flow can be addressed separately. https://claude.ai/code/session_016aL3vh5nHHa2QbmAxyFCGy

coderabbitai

🧹 Nitpick comments (1)

plugins/SLASHED-for-WP/integrations/bricks/editor-app/src/components/ColorPanel.svelte (1)
202-203: 💤 Low value

Both onclick and onmousedown propagation stops are present.

The review stack context describes this change as "Switch panel root event handling from onclick propagation stop to onmousedown propagation stop", which suggests onclick should have been replaced. However, both handlers are now present on the panel root.

Functionally, this is safe and defensive—it stops propagation for both event types regardless of whether Bricks listens to click or mousedown events. If this dual-handler approach is intentional for maximum compatibility, consider briefly documenting it in a comment. Otherwise, if onclick is no longer needed, it could be removed to match the stated simplification intent.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@plugins/SLASHED-for-WP/integrations/bricks/editor-app/src/components/ColorPanel.svelte`
around lines 202 - 203, The panel root in ColorPanel.svelte currently has both
onclick and onmousedown propagation stops but the change intent was to switch
from onclick to onmousedown; remove the redundant onclick={(e) =>
e.stopPropagation()} attribute so only onmousedown={(e) => e.stopPropagation()}
remains on the panel root (or, if you intended to keep both for compatibility,
replace removal with a one-line comment above the handlers explaining the
deliberate dual handling); locate these attributes in the ColorPanel.svelte
component and update accordingly.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In
`@plugins/SLASHED-for-WP/integrations/bricks/editor-app/src/components/ColorPanel.svelte`:
- Around line 202-203: The panel root in ColorPanel.svelte currently has both
onclick and onmousedown propagation stops but the change intent was to switch
from onclick to onmousedown; remove the redundant onclick={(e) =>
e.stopPropagation()} attribute so only onmousedown={(e) => e.stopPropagation()}
remains on the panel root (or, if you intended to keep both for compatibility,
replace removal with a one-line comment above the handlers explaining the
deliberate dual handling); locate these attributes in the ColorPanel.svelte
component and update accordingly.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 89de1537-8cc2-4dad-8714-29af4cbad28e

📥 Commits

Reviewing files that changed from the base of the PR and between 9060438 and ed77906.

📒 Files selected for processing (4)

plugins/SLASHED-for-WP/integrations/bricks/assets/editor-app/app.js
plugins/SLASHED-for-WP/integrations/bricks/editor-app/src/components/ColorPanel.svelte
plugins/SLASHED-for-WP/integrations/bricks/editor-app/src/components/ColorSwatch.svelte
plugins/SLASHED-for-WP/integrations/bricks/editor-app/src/lib/color-swatches.js

💤 Files with no reviewable changes (2)

plugins/SLASHED-for-WP/integrations/bricks/editor-app/src/components/ColorSwatch.svelte
plugins/SLASHED-for-WP/integrations/bricks/editor-app/src/lib/color-swatches.js

jackgranatowski force-pushed the claude/bricks-border-sf-color-bug-f33Qj branch from 3507b00 to ed77906 Compare June 3, 2026 21:25

coderabbitai Bot reviewed Jun 3, 2026

View reviewed changes

jackgranatowski merged commit 422603a into main Jun 3, 2026
9 checks passed

coderabbitai Bot mentioned this pull request Jun 5, 2026

Fix SF color button state sync and touch event handling #232

Merged

Navigation Menu

Uh oh!

revert(bricks): remove SF colour border/box-shadow fixes - #212

revert(bricks): remove SF colour border/box-shadow fixes#212
jackgranatowski merged 1 commit into
mainfrom
claude/bricks-border-sf-color-bug-f33Qj

jackgranatowski commented Jun 3, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 3, 2026 •

edited

Loading

Walkthrough

Changes

Possibly related PRs

Estimated code review effort

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

jackgranatowski commented Jun 3, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why

Test plan

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Possibly related PRs

Estimated code review effort

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jackgranatowski commented Jun 3, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 3, 2026 •

edited

Loading